Hi @vishal-chdhry. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

@aliok I had the following doubts

• There are a few references of DeadLetterChannel in v1beta1 as well

  eventing/pkg/apis/duck/v1beta1/delivery_types.go

  Lines 113 to 117 in 8b7551c

  	type DeliveryStatus struct {
  	// DeadLetterChannel is a KReference that is the reference to the native, platform specific channel
  	// where failed events are sent to.
  	// +optional
  	DeadLetterChannel *duckv1.KReference `json:"deadLetterChannel,omitempty"`

  eventing/pkg/apis/duck/v1beta1/channelable_types.go

  Lines 63 to 66 in 8b7551c

  	// DeadLetterChannel is a KReference and is set by the channel when it supports native error handling via a channel
  	// Failed messages are delivered here.
  	// +optional
  	DeadLetterChannel *duckv1.KReference `json:"deadLetterChannel,omitempty"`

  but these two are not deprecated, should i remove them too? I have verified that these two are not in use.
• How do I send the emails to knative users email list?
• What is the command for codegen, i could not find the makefile

/hold

@vishal-chdhry

There are a few references of DeadLetterChannel in v1beta1 as well

No, the ones in v1beta1 need to stay.

How do I send the emails to knative users email list?

Here's the link to the Google Group: https://groups.google.com/g/knative-users
There is also knative-dev: https://groups.google.com/g/knative-dev

I think sending an email to both makes more sense.

What is the command for codegen, i could not find the makefile

You can run ./hack/update-codegen.sh. That will regenerate the zz_generated.deepcopy.go file relevant here.

/assign

Assigning myself as a reviewer

/ok-to-test

Ah, one more thing. Can you also fill the Release Note section in the PR description that this field is removed? It would be nice to note that in the release notes. For example, #6665 has a release note that is shown in the 1.9.2 release

Codecov Report

Patch and project coverage have no change.

Comparison is base (8b7551c) 80.53% compared to head (936956c) 80.53%.

@@           Coverage Diff           @@
##             main    #6722   +/-   ##
=======================================
  Coverage   80.53%   80.53%           
=======================================
  Files         236      236           
  Lines       12100    12100           
=======================================
  Hits         9745     9745           
  Misses       1867     1867           
  Partials      488      488           

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

There are some failures in some Github actions.

• Downstream / Unit Test (eventing-natss, knative-sandbox): We should simply create a ticket in https://github.com/knative-sandbox/eventing-natss, referencing this PR and the ticket
• KinD e2e tests / e2e tests (v1.25.3, ./test/experimental): Needs to be checked
• KinD e2e tests / e2e tests (v1.25.3, ./test/e2e): Needs to be checked
• KinD e2e tests / e2e tests (v1.25.3, ./test/rekt/...): Needs to be checked

Let's first do the sanity check for the last 3. Are they passing on other PRs? Or, are they really getting broken by this PR?

we have failing tests in this PR as well #6696

Are they failing the same way?
Also KinD e2e tests / e2e tests (v1.25.3, ./test/e2e) is not failing there. I am triggering these again now

@aliok Yes both have the same error

runtime.main
	/opt/hostedtoolcache/go/1.18.10/x64/src/runtime/proc.go:250
--- PASS: ExampleWithRetry (0.00s)
PASS
ok  	knative.dev/eventing/test/rekt/resources/trigger	0.271s
FAIL
Error: Process completed with exit code 1.

ok, then we're all good. Can you unhold after a week from your email you sent to the community?

/lgtm
/approve

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aliok, Vishal-Chdhry

The full list of commands accepted by this bot can be found here.

The pull request process is described here

I am OK w/ this direct removal, since the API has been deprecated in 2021, and was meant to be removed. So we are OK in NOT bumping to v2alpha1 ... 😄

Usually we should do the migration etc.

Note: NATSS downstream has issues, we just ignore this.

Thanks for the contribution, @vishal-chdhry

@matzew I have opened an issue in natss regarding this knative-extensions/eventing-natss#388.
Should I go ahead and remove the references to DeadLetterChannel there as well??

/cc aliok
/uncc

Should I go ahead and remove the references to DeadLetterChannel there as well??

@vishal-chdhry I don't know the latest status of Nats repository. If you think you can fix the issue there, feel free and give it a try.

/unhold